[fix] Make ID the source of truth for vpc and subnet names#807
Merged
komer3 merged 2 commits intolinode:mainfrom Jul 23, 2025
Merged
[fix] Make ID the source of truth for vpc and subnet names#807komer3 merged 2 commits intolinode:mainfrom
komer3 merged 2 commits intolinode:mainfrom
Conversation
cbbond
force-pushed
the
dont-filter-by-vpc-label
branch
from
da4d3b2 to
125dc8d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #807 +/- ##
==========================================
+ Coverage 63.16% 63.25% +0.08%
==========================================
Files 71 71
Lines 7292 7304 +12
==========================================
+ Hits 4606 4620 +14
+ Misses 2415 2414 -1
+ Partials 271 270 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cbbond
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where CAPL (Cluster API Provider Linode) fails to properly reconcile VPCs and subnets when their labels are changed outside of CAPL. The fix makes resource IDs the primary source of truth instead of relying on label matching.
Key changes:
- Removes label-based filtering when looking up existing VPCs, using only VPC ID
- Implements dual mapping for subnets (by both ID and label) to handle label changes gracefully
- Updates subnet reconciliation logic to prefer ID-based lookups with label fallback
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/controller/linodevpc_controller_helpers.go | Core logic changes to remove label filtering for VPCs and implement dual subnet mapping |
| internal/controller/linodevpc_controller_test.go | New test case covering VPC and subnet label changes |
Comments suppressed due to low confidence (2)
internal/controller/linodevpc_controller_helpers.go:88
- [nitpick] The variable name 'subnetsByLabel' uses camelCase inconsistent with Go naming conventions. It should be 'subnetsByLabel' or consider 'subnetConfigByLabel' for clarity.
subnetsByLabel := make(map[string]SubnetConfig, len(vpc.Subnets))
internal/controller/linodevpc_controller_helpers.go:89
- The variable name 'subnetsById' should be 'subnetsByID' to follow Go naming conventions where 'ID' should be capitalized.
subnetsById := make(map[int]SubnetConfig, len(vpc.Subnets))
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
We currently filter the Linode API's ListVPCs response by both VPC ID and label when deciding whether to create a new VPC or update an existing one. This PR removes the latter filter and only checks for existing VPCs that match the ID found on the LinodeVPC resource. VPC ID is immutable and should be a sufficient filter for finding VPCs. Without this fix, any change to the VPC's label outside of CAPL will result in CAPL trying to create a new VPC when the underlying resource still exists.
Similarly, we currently build a map of subnets by label based on a list of VPC subnets we get from the Linode API. This method is vulnerable to the same issue in which any change to a subnet label after creation will prevent CAPL from reconciling the subnet further. This PR fixes this issue by building two maps that are keyed by both label and subnet ID. The code defaults to ID where possible but falls back to the label when the subnet ID is not yet populated (i.e. the first time we are reconciling a new subnet).
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged): N/ASpecial notes for your reviewer:
TODOs: